✨ Add Quantum IPO Pass to mqt-cc - #1970
Conversation
…toolkit/core into mlir/measurement-lifting
Signed-off-by: Damian Rovara <93778306+DRovara@users.noreply.github.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This commit was authored by Claude Opus 5.0
Signed-off-by: Damian Rovara <93778306+DRovara@users.noreply.github.com>
Signed-off-by: Damian Rovara <93778306+DRovara@users.noreply.github.com>
…e into mlir/ipo-pass
Signed-off-by: Damian Rovara <93778306+DRovara@users.noreply.github.com>
This commit was assisted by Claude Opus 5.0
…e into mlir/ipo-pass
|
@coderabbitai full review |
✅ Action performedFull review finished. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change adds unsized QASM3 single-qubit support and introduces QCO APIs and passes for function construction, linear quantum-resource tracking, call specialization, argument promotion, auxiliary-qubit hoisting, and function-boundary gate commutation. ChangesQASM3 single-qubit typing
QCO quantum interprocedural optimization
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant QuantumIPO
participant Callee
participant CallSite
participant QuantumArgumentPromotion
participant AuxiliaryQubitHoisting
participant BoundaryCommutation
QuantumIPO->>Callee: clone and specialize for operand context
QuantumIPO->>CallSite: redirect call to specialized callee
QuantumIPO->>QuantumArgumentPromotion: promote eligible tensor arguments
QuantumArgumentPromotion->>CallSite: rewrite extracts, inserts, and results
QuantumIPO->>AuxiliaryQubitHoisting: hoist eligible auxiliary allocations
AuxiliaryQubitHoisting->>CallSite: rewrite allocation, call, and reset flow
QuantumIPO->>BoundaryCommutation: remove matching boundary gates
BoundaryCommutation->>CallSite: retarget call to specialized callee
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 30
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mlir/lib/Dialect/QTensor/Utils/TensorIterator.cpp (1)
151-164: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReturn
nullptrforfunc::ReturnOpinTensorIterator::tensor().
forward()already marksfunc::ReturnOpas final, buttensor()still returns the live operand when the iterator stops there.func::ReturnOphas no results, so it belongs in the no-OpResultcase alongsideDeallocOpand the yield/condition ops.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@mlir/lib/Dialect/QTensor/Utils/TensorIterator.cpp` around lines 151 - 164, Update TensorIterator::tensor() to return nullptr when the iterator is positioned at a func::ReturnOp, treating it as a no-OpResult terminal operation like DeallocOp and the yield/condition operations. Preserve the existing defining-operation handling and ensure forward()’s final-state behavior remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mlir/include/mlir/Dialect/QCO/Transforms/Passes.h`:
- Line 13: Add the mlir/IR/SymbolTable.h include to Passes.h so the SymbolTable
reference declaration is available from the header.
- Around line 37-40: Rename all ModuleOp parameters named module to moduleOp and
update every corresponding use: the three declarations in
mlir/include/mlir/Dialect/QCO/Transforms/Passes.h#L37-L40;
runAuxiliaryQubitHoisting and its uses in
mlir/lib/Dialect/QCO/Transforms/Optimizations/AuxiliaryQubitHoisting.cpp#L262-L280;
runQuantumFunctionBoundaryCommutation and its use in
mlir/lib/Dialect/QCO/Transforms/Optimizations/QuantumFunctionBoundaryCommutation.cpp#L102-L105;
runQuantumIPOPass and runCanonicalizerPass in
mlir/unittests/Dialect/QCO/Transforms/Optimizations/test_qco_quantum_ipo.cpp#L63-L77;
and runQuantumArgumentPromotion in
mlir/lib/Dialect/QCO/Transforms/Optimizations/QuantumArgumentPromotion.cpp#L315.
Keep declarations, definitions, and call behavior consistent while eliminating
shadowing of the module fixture.
In `@mlir/include/mlir/Dialect/QCO/Transforms/Passes.td`:
- Around line 309-312: Expand the `QuantumIPO` `description` to document its
four transformations: context-sensitive call specialization, quantum argument
promotion, auxiliary-qubit hoisting, and function-boundary commutation. Include
that it skips public functions, declarations, and recursive functions; depends
on the QCO convention matching each qubit result to the corresponding operand;
and runs `runQuantumFunctionBoundaryCommutation` exactly two times rather than
to a fixed point. Align the description indentation with neighboring pass
definitions.
In `@mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp`:
- Around line 1485-1503: Validate function signatures eagerly in
QCOProgramBuilder.cpp: at lines 1485-1503, update call to compare
operands.getTypes() with funcOp.getArgumentTypes() and reportFatalUsageError
before linear-tracking updates; at lines 1450-1475, update endFunction to
compare returnValues.getTypes() with the enclosing function’s getResultTypes()
and reportFatalUsageError before creating func::ReturnOp.
- Around line 1385-1387: Update QCOProgramBuilder::getQubitTensorType to reject
size <= 0 with llvm::reportFatalUsageError, matching allocQubitRegister and
allocClassicalBitRegister, before constructing the RankedTensorType.
- Around line 1450-1475: Update endFunction to eagerly validate returnValues
against the resultTypes recorded by startFunction before creating
func::ReturnOp: reportFatalUsageError when the arity differs or any
corresponding value type differs from its declared result type. Keep the
existing qubit/tensor validation and cleanup checks, and perform this validation
at the endFunction call site before ReturnOp creation.
- Around line 1414-1419: In startFunction, check
SymbolTable::lookupSymbolIn(module, name) before func::FuncOp::create and reject
the operation when a function with the same name already exists. Preserve the
existing insertion point, function creation, and private visibility behavior for
unique names.
In `@mlir/lib/Dialect/QCO/IR/QCOOps.cpp`:
- Around line 485-507: Update the comments in QCOInlinerInterface to
consistently refer to the QCO dialect instead of the qc dialect, including all
four occurrences; leave the inlining behavior and method implementations
unchanged.
In `@mlir/lib/Dialect/QCO/Transforms/Optimizations/AuxiliaryQubitHoisting.cpp`:
- Around line 141-175: Replace the recursive traversal in isRecursiveHelper with
an iterative worklist or llvm::scc_iterator-based cycle check so recursion depth
cannot exhaust the stack. Preserve isRecursive’s behavior of detecting cycles
reachable from the function’s callees while continuing to avoid treating the
function as recursive solely through its initial node.
- Around line 177-182: Update tryAuxiliaryQubitHoisting to collect AllocOp
instances before erasing or otherwise mutating them, rather than modifying
operations during funcOp.walk. Apply the same collect-then-mutate approach for
func::ReturnOp handling, or preferably update return operands in place with
returnOp->setOperands(...) and avoid replacing or erasing the terminator; ensure
the return walk continues by closing the lambda loop instead of returning early.
- Around line 98-110: Update the call-handling logic around the visible
func::CallOp branch to avoid indexing results with an operand index: only
advance currentValue when a matching operand has a corresponding result, and
otherwise terminate or leave the traversal safely without accessing an
out-of-bounds result. Also handle the no-matching-operand case so the enclosing
while loop cannot repeat indefinitely, while preserving the existing linear
value propagation for valid operand/result pairs.
- Around line 262-285: Update runAuxiliaryQubitHoisting to rename its ModuleOp
parameter from module to moduleOp, remove the empty per-function
RewritePatternSet and applyPatternsGreedily call, and eliminate the
std::runtime_error throw. If reuse-qubit patterns remain required, populate and
apply them once after processing all hoistingCandidates, returning LogicalResult
on failure so QuantumIPO can call signalPassFailure; remove now-unused headers.
In `@mlir/lib/Dialect/QCO/Transforms/Optimizations/QuantumArgumentPromotion.cpp`:
- Around line 84-88: Update the user-traversal logic in canPromoteArgument to
reject unknown operations instead of following getResult(0) whenever a
single-result user is not explicitly recognized as preserving the qubit chain.
Mirror the existing unknown-user rejection used later in canPromoteArgument,
while retaining result-0 traversal only for supported operation types.
- Around line 283-303: Use the caller’s call-site location for all generated
promotion operations in this block: replace loc with callLoc when creating the
extract and insert operations, their ConstantIndexOp indices, and the
func::CallOp. Keep loc for callee-related processing and preserve the existing
operand and tensor transformation logic.
- Around line 229-246: Fix returnedQubits construction in the argument-promotion
flow so pass-through slots do not retain the erased extract result: build each
returned qubit after the rewiring/erasure using the surviving replacement value,
or substitute newArgs[i] directly when the insert scalar aliases the extract
result. Add a regression test covering a callee that extracts and immediately
reinserts an element without applying a gate.
In
`@mlir/lib/Dialect/QCO/Transforms/Optimizations/QuantumFunctionBoundaryCommutation.cpp`:
- Around line 23-25: Replace the std::unordered_map used by the quantum function
boundary optimization with an appropriate LLVM map container, preferably
llvm::StringMap, so lookups in the affected optimization logic accept StringRef
without creating temporary std::string keys. Update the associated include and
map access/types while preserving the existing key-value behavior.
- Around line 36-46: Update doOpsCancel to require both operations report
getNumQubits() == 1 before accepting the matching self-inverse gate types,
preserving the existing name and isa<XOp, YOp, ZOp, HOp> checks. This makes the
single-qubit contract explicit for the caller’s getInputQubit(0) usage.
- Around line 102-113: Rename the runQuantumFunctionBoundaryCommutation
parameter from module to moduleOp, and collect all func::CallOp values before
processing them. Replace the mutating module.walk processing with a two-phase
flow: gather calls using moduleOp, then iterate the collected calls and invoke
tryBoundaryCommutation so erasing operations cannot invalidate the walk.
- Around line 81-99: Key previousSpecializations by both the callee name and
parameter index so each cached specialization corresponds to the boundary gate
removed from that specific argument. Update the cache declaration and all
lookup, insertion, and access sites in the specialization flow around
copyFunction, preserving distinct redirected callees for different parameters.
Add a unit test covering one two-qubit callee specialized at separate call sites
for each parameter and verify both calls use the correct specialization.
In `@mlir/lib/Dialect/QCO/Transforms/Optimizations/QuantumIPO.cpp`:
- Around line 261-265: The specialization check in the function-name handling
around the visible funcOp logic is too broad and angle-insensitive. Remove the
funcOp.getName().contains(suffix) marker check and use a discardable attribute
on each cloned function to record the specific rotation specialization, reusing
rotationSpecializations’ name/operand/angle identity to distinguish different
angles and operands.
- Around line 280-285: Preserve and restore the shared PatternRewriter insertion
point around the update in the specialization routine containing newParameter
and the arith::ConstantOp. Save the current insertion point before
setInsertionPointToStart, perform constant creation and replacement, then
restore the saved point so trySpecializeZero and trySpecializePlus remain
independent of execution order.
- Around line 328-336: Update runOnOperation so it returns immediately after
signalPassFailure when applyPatternsGreedily fails, preventing subsequent
transformations from running on invalid IR. Remove the duplicated
runQuantumFunctionBoundaryCommutation call, keeping a single invocation with the
existing arguments.
- Around line 11-13: Remove the IDE-generated “Created by” comment at the top of
QuantumIPO.cpp, and add a concise top-level `@brief` documenting the purpose of
the quantum IPO pass, following the documentation style used in
QuantumArgumentPromotion.cpp.
- Around line 95-101: Update operationIsNopOnZero’s control lookup for CtrlOp to
use LLVM’s range-based llvm::is_contained helper with ctrl.getControlsIn()
instead of std::find and explicit iterators. Leave the existing zero-operation
checks and TODO marker unchanged.
- Around line 192-205: Update the loop around operationIsNopOnZero and the
newUser cast to handle ResetOp before accessing UnitaryOpInterface: replace the
reset output with its input qubit and erase the reset operation, then continue
the optimization safely. Alternatively, restrict this path to unitary users so
non-unitary operations are not cast or dereferenced.
- Around line 56-66: Update copyFunction to remove the unused PatternRewriter
parameter, insertion guard, and setInsertionPointAfter call; clone the function
and set its name directly while detached, then adjust all callers to use the new
signature before symbolTable.insert attaches the clone.
In
`@mlir/unittests/Dialect/QCO/Transforms/Optimizations/test_qco_quantum_ipo.cpp`:
- Around line 102-131: Add a QCO quantum IPO zero-specialization test alongside
specializeZeroArgumentDropsDiagonalGate where the callee’s leading operation is
reset on its qubit argument and the caller passes a freshly allocated qubit.
Build the reference with the original reset-containing callee retained and a
zero-argument specialization that forwards the argument without the reset, then
invoke and sink the specialized result to validate the ResetOp path.
- Around line 546-582: Add a separate promotion test alongside
promoteTensorArgumentToQubitArgument where the callee extracts element 0 from
its tensor argument and immediately re-inserts that same qubit without applying
a gate, returning only the rebuilt tensor. Build the reference with the promoted
qubit passed through directly, then preserve the caller’s
extract/call/insert/deallocate structure and verify with
expectModuleMatchesReference.
In `@src/qasm3/passes/TypeCheckPass.cpp`:
- Around line 378-388: In visitAssignmentStatement(), immediately after
evaluating the right-hand expression and obtaining exprTy, check exprTy.isError
and return before dereferencing exprTy.type. Preserve the existing assignment
type-checking flow for non-error inferred types, including results from
visitMeasureExpression().
In `@test/ir/test_qasm3_parser.cpp`:
- Line 209: Change the test declaration from sized `qubit[1] q;` to unsized
`qubit q;` so it exercises the `UnsizedTy::SingleQubit` path through the parser,
importer, and type checker, and add a separate measurement test case covering
that unsized qubit behavior.
---
Outside diff comments:
In `@mlir/lib/Dialect/QTensor/Utils/TensorIterator.cpp`:
- Around line 151-164: Update TensorIterator::tensor() to return nullptr when
the iterator is positioned at a func::ReturnOp, treating it as a no-OpResult
terminal operation like DeallocOp and the yield/condition operations. Preserve
the existing defining-operation handling and ensure forward()’s final-state
behavior remains unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 039cc50e-80d3-428f-8a86-805ac0eb0dfb
📒 Files selected for processing (20)
include/mqt-core/qasm3/Types.hppmlir/include/mlir/Dialect/QCO/Builder/QCOProgramBuilder.hmlir/include/mlir/Dialect/QCO/Transforms/Passes.hmlir/include/mlir/Dialect/QCO/Transforms/Passes.tdmlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cppmlir/lib/Dialect/QCO/IR/QCOOps.cppmlir/lib/Dialect/QCO/Transforms/Optimizations/AuxiliaryQubitHoisting.cppmlir/lib/Dialect/QCO/Transforms/Optimizations/QuantumArgumentPromotion.cppmlir/lib/Dialect/QCO/Transforms/Optimizations/QuantumFunctionBoundaryCommutation.cppmlir/lib/Dialect/QCO/Transforms/Optimizations/QuantumIPO.cppmlir/lib/Dialect/QTensor/Utils/CMakeLists.txtmlir/lib/Dialect/QTensor/Utils/TensorIterator.cppmlir/lib/Support/IRVerification.cppmlir/unittests/Dialect/QCO/IR/test_qco_ir.cppmlir/unittests/Dialect/QCO/Transforms/Optimizations/CMakeLists.txtmlir/unittests/Dialect/QCO/Transforms/Optimizations/test_qco_quantum_ipo.cppsrc/qasm3/Importer.cppsrc/qasm3/Parser.cppsrc/qasm3/passes/TypeCheckPass.cpptest/ir/test_qasm3_parser.cpp
|
|
||
| #pragma once | ||
|
|
||
| #include <mlir/IR/BuiltinOps.h> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Show the include block of the QCO Transforms Passes.h header.
fd -p 'mlir/include/mlir/Dialect/QCO/Transforms/Passes.h' --exec sed -n '1,45p' {}Repository: munich-quantum-toolkit/core
Length of output: 1448
Include mlir/IR/SymbolTable.h for SymbolTable.
Passes.h now declares SymbolTable& symbolTable, so the header must provide that declaration.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mlir/include/mlir/Dialect/QCO/Transforms/Passes.h` at line 13, Add the
mlir/IR/SymbolTable.h include to Passes.h so the SymbolTable reference
declaration is available from the header.
| void runQuantumArgumentPromotion(ModuleOp module); | ||
| void runAuxiliaryQubitHoisting(ModuleOp module); | ||
| void runQuantumFunctionBoundaryCommutation(ModuleOp module, | ||
| SymbolTable& symbolTable); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
mlir::ModuleOp parameters are named module across the new IPO surface. The coding guidelines forbid module as a C++ variable or parameter name and require moduleOp. The new declarations and their definitions all carry the forbidden name, so the rename must stay consistent between the header and every definition.
mlir/include/mlir/Dialect/QCO/Transforms/Passes.h#L37-L40: rename theModuleOp moduleparameter tomoduleOpin all three declarations.mlir/lib/Dialect/QCO/Transforms/Optimizations/AuxiliaryQubitHoisting.cpp#L262-L262: rename theModuleOp moduleparameter ofrunAuxiliaryQubitHoistingtomoduleOpand update its uses at lines 264, 266, 279, and 280.mlir/lib/Dialect/QCO/Transforms/Optimizations/QuantumFunctionBoundaryCommutation.cpp#L102-L102: rename theModuleOp moduleparameter ofrunQuantumFunctionBoundaryCommutationtomoduleOpand update its use at line 105.mlir/unittests/Dialect/QCO/Transforms/Optimizations/test_qco_quantum_ipo.cpp#L63-L77: rename theModuleOp moduleparameters ofrunQuantumIPOPassandrunCanonicalizerPasstomoduleOp, which also removes the shadowing of themodulefixture member at line 44.
mlir/lib/Dialect/QCO/Transforms/Optimizations/QuantumArgumentPromotion.cpp at line 315 declares runQuantumArgumentPromotion(ModuleOp module) with the same name; apply the rename there as well.
Based on coding guidelines: "Do not use module as a C++ variable or parameter name; use moduleOp for mlir::ModuleOp values."
📍 Affects 4 files
mlir/include/mlir/Dialect/QCO/Transforms/Passes.h#L37-L40(this comment)mlir/lib/Dialect/QCO/Transforms/Optimizations/AuxiliaryQubitHoisting.cpp#L262-L262mlir/lib/Dialect/QCO/Transforms/Optimizations/QuantumFunctionBoundaryCommutation.cpp#L102-L102mlir/unittests/Dialect/QCO/Transforms/Optimizations/test_qco_quantum_ipo.cpp#L63-L77
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mlir/include/mlir/Dialect/QCO/Transforms/Passes.h` around lines 37 - 40,
Rename all ModuleOp parameters named module to moduleOp and update every
corresponding use: the three declarations in
mlir/include/mlir/Dialect/QCO/Transforms/Passes.h#L37-L40;
runAuxiliaryQubitHoisting and its uses in
mlir/lib/Dialect/QCO/Transforms/Optimizations/AuxiliaryQubitHoisting.cpp#L262-L280;
runQuantumFunctionBoundaryCommutation and its use in
mlir/lib/Dialect/QCO/Transforms/Optimizations/QuantumFunctionBoundaryCommutation.cpp#L102-L105;
runQuantumIPOPass and runCanonicalizerPass in
mlir/unittests/Dialect/QCO/Transforms/Optimizations/test_qco_quantum_ipo.cpp#L63-L77;
and runQuantumArgumentPromotion in
mlir/lib/Dialect/QCO/Transforms/Optimizations/QuantumArgumentPromotion.cpp#L315.
Keep declarations, definitions, and call behavior consistent while eliminating
shadowing of the module fixture.
Source: Coding guidelines
| let summary = "Quantum Interprocedural Optimization Pass"; | ||
| let description = [{ | ||
| Performs interprocedural optimizations on quantum functions. | ||
| }]; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Expand the QuantumIPO description to match its actual scope.
The description is one generic sentence. The pass runs four distinct transformations: context-sensitive call specialization, quantum argument promotion, auxiliary-qubit hoisting, and function-boundary commutation. It also has real limitations that a user cannot discover from this text:
- It skips public functions and declarations.
- It skips recursive functions.
- It relies on the QCO calling convention that the i-th qubit result of a call corresponds to the i-th qubit operand.
- It runs
runQuantumFunctionBoundaryCommutationa fixed two times rather than to a fixed point.
Every neighbouring pass in this file documents scope and limitations at this level. Also align the description indentation with the surrounding passes.
As per coding guidelines: "When changing MLIR passes, pipelines, or command-line options, align summaries and descriptions with actual scope, defaults, supported operation shapes, limitations, failure modes, and out-of-scope behavior."
📝 Proposed description skeleton
def QuantumIPO : Pass<"quantum-ipo", "mlir::ModuleOp"> {
let summary = "Quantum Interprocedural Optimization Pass";
let description = [{
- Performs interprocedural optimizations on quantum functions.
- }];
+ Performs interprocedural optimizations across `func.func` boundaries by
+ running, in order:
+
+ - Context-sensitive specialization of `func.call` sites for recognized
+ qubit states and constant angles.
+ - Quantum argument promotion, turning eligible qubit-tensor arguments into
+ scalar qubit arguments.
+ - Auxiliary-qubit hoisting, moving callee-internal allocations to the
+ caller.
+ - Quantum function-boundary commutation, removing matching self-inverse
+ gates across a call.
+
+ Limitations: public functions and declarations are not transformed;
+ recursive functions are skipped; the pass relies on the QCO calling
+ convention that the i-th qubit result of a call corresponds to its i-th
+ qubit operand; boundary commutation runs a fixed number of iterations
+ rather than to a fixed point.
+ }];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let summary = "Quantum Interprocedural Optimization Pass"; | |
| let description = [{ | |
| Performs interprocedural optimizations on quantum functions. | |
| }]; | |
| let summary = "Quantum Interprocedural Optimization Pass"; | |
| let description = [{ | |
| Performs interprocedural optimizations across `func.func` boundaries by | |
| running, in order: | |
| - Context-sensitive specialization of `func.call` sites for recognized | |
| qubit states and constant angles. | |
| - Quantum argument promotion, turning eligible qubit-tensor arguments into | |
| scalar qubit arguments. | |
| - Auxiliary-qubit hoisting, moving callee-internal allocations to the | |
| caller. | |
| - Quantum function-boundary commutation, removing matching self-inverse | |
| gates across a call. | |
| Limitations: public functions and declarations are not transformed; | |
| recursive functions are skipped; the pass relies on the QCO calling | |
| convention that the i-th qubit result of a call corresponds to its i-th | |
| qubit operand; boundary commutation runs a fixed number of iterations | |
| rather than to a fixed point. | |
| }]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mlir/include/mlir/Dialect/QCO/Transforms/Passes.td` around lines 309 - 312,
Expand the `QuantumIPO` `description` to document its four transformations:
context-sensitive call specialization, quantum argument promotion,
auxiliary-qubit hoisting, and function-boundary commutation. Include that it
skips public functions, declarations, and recursive functions; depends on the
QCO convention matching each qubit result to the corresponding operand; and runs
`runQuantumFunctionBoundaryCommutation` exactly two times rather than to a fixed
point. Align the description indentation with neighboring pass definitions.
Source: Coding guidelines
| Type QCOProgramBuilder::getQubitTensorType(const int64_t size) { | ||
| return RankedTensorType::get({size}, getQubitType()); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Reject non-positive sizes in getQubitTensorType.
allocQubitRegister and allocClassicalBitRegister both reject size <= 0 with llvm::reportFatalUsageError. getQubitTensorType accepts any value. A negative size produces an invalid static dimension and triggers an MLIR assertion far away from the call site.
Add the same guard so the failure points at the builder call.
🛡️ Proposed guard
Type QCOProgramBuilder::getQubitTensorType(const int64_t size) {
+ if (size <= 0) {
+ llvm::reportFatalUsageError("Size must be positive");
+ }
return RankedTensorType::get({size}, getQubitType());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Type QCOProgramBuilder::getQubitTensorType(const int64_t size) { | |
| return RankedTensorType::get({size}, getQubitType()); | |
| } | |
| Type QCOProgramBuilder::getQubitTensorType(const int64_t size) { | |
| if (size <= 0) { | |
| llvm::reportFatalUsageError("Size must be positive"); | |
| } | |
| return RankedTensorType::get({size}, getQubitType()); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp` around lines 1385 - 1387,
Update QCOProgramBuilder::getQubitTensorType to reject size <= 0 with
llvm::reportFatalUsageError, matching allocQubitRegister and
allocClassicalBitRegister, before constructing the RankedTensorType.
| setInsertionPointToEnd(cast<ModuleOp>(module).getBody()); | ||
| auto funcOp = | ||
| func::FuncOp::create(*this, name, getFunctionType(argTypes, resultTypes)); | ||
| // The interprocedural passes only consider functions that are not externally | ||
| // visible, so additional functions are private by default. | ||
| funcOp.setPrivate(); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect startFunction callers and any existing symbol-uniqueness handling.
set -euo pipefail
# Find all startFunction call sites.
rg -nP -C4 '\bstartFunction\s*\(' --type=cpp --type=cc
# Check whether SymbolTable::insert is used anywhere in the QCO builder.
fd -t f 'QCOProgramBuilder.cpp' | xargs rg -nP -C3 'SymbolTable::(insert|lookupSymbolIn)'Repository: munich-quantum-toolkit/core
Length of output: 196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== QCOProgramBuilder outline =="
ast-grep outline mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp --match startFunction --view expanded || true
echo "== startFunction definition =="
python3 - <<'PY'
from pathlib import Path
p=Path('mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp')
lines=p.read_text().splitlines()
for i,l in enumerate(lines):
if 'startFunction' in lines[max(0,i-2):i+1]:
start=max(0,i-18)
end=min(len(lines),i+80)
print(f'-- lines {start+1}-{end} --')
for n in range(start,end):
print(f'{n+1:5}: {lines[n]}')
print()
PY
echo "== all startFunction call sites =="
rg -nP -C4 '\bstartFunction\s*\(' -g '*.{cc,cpp,cxx,h,hpp,hh}'
echo "== SymbolTable usage in QCOProgramBuilder =="
rg -nP -C3 'SymbolTable::(insert|lookupSymbolIn)' mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp
echo "== uses of func::FuncOp::create =="
rg -nP -C2 'func::FuncOp::create|FuncOp::create' -g '*.{cc,cpp,cxx,h,hpp,hh}'Repository: munich-quantum-toolkit/core
Length of output: 388
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== startFunction definition =="
python3 - <<'PY'
from pathlib import Path
p=Path('mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp')
lines=p.read_text().splitlines()
for i,l in enumerate(lines):
if 'SmallVector<Value> QCOProgramBuilder::startFunction(StringRef name,' in l:
start=max(0,i-15)
end=min(len(lines),i+100)
print(f'-- lines {start+1}-{end} --')
for n in range(start,end):
print(f'{n+1:5}: {lines[n]}')
print()
PY
echo "== all startFunction call sites =="
rg -nP -C4 '\bstartFunction\s*\(' -g '*.cc' -g '*.cpp' -g '*.cxx' -g '*.h' -g '*.hpp' -g '*.hh' .
echo "== SymbolTable usage in QCOProgramBuilder =="
rg -nP -C3 'SymbolTable::(insert|lookupSymbolIn)' mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp
echo "== uses of func::FuncOp::create =="
rg -nP -C2 'func::FuncOp::create|FuncOp::create' -g '*.cc' -g '*.cpp' -g '*.cxx' -g '*.h' -g '*.hpp' -g '*.hh' .Repository: munich-quantum-toolkit/core
Length of output: 50386
🌐 Web query:
MLIR func FuncOp create SymbolTable insert SymbolTable::lookupSymbolIn
💡 Result:
In MLIR, a SymbolTable manages operations that define symbols (e.g., func.func) [1][2]. To work with these symbols, you use the mlir::SymbolTable class and its associated static utility methods [3][4]. Creating and Managing a SymbolTable Operations that function as containers for symbols must implement the OpTrait::SymbolTable trait [1][2]. You can create an instance of the SymbolTable class by passing the operation that holds the symbols to its constructor [5][6]: mlir::SymbolTable symbolTable(symbolTableOp); Once the object is created, you can insert new symbol operations into the container [6]: symbolTable.insert(newSymbolOp); Looking Up Symbols The mlir::SymbolTable class provides static utility methods to look up symbols within an operation that has the SymbolTable trait [3][4]. These methods do not require you to explicitly instantiate a SymbolTable object [4][7]. The primary method is lookupSymbolIn, which returns the Operation* matching a given symbol name [3][4]. It is overloaded to accept different types of symbol identifiers [4]: Operation *op = mlir::SymbolTable::lookupSymbolIn(symbolTableOp, StringAttr::get(context, "symbol_name")); Operation *op = mlir::SymbolTable::lookupSymbolIn(symbolTableOp, "symbol_name"); Operation *op = mlir::SymbolTable::lookupSymbolIn(symbolTableOp, symbolRefAttr); Key Points: - Container Requirements: The container operation passed to lookupSymbolIn or the SymbolTable constructor must possess the OpTrait::SymbolTable trait [3][4][2]. - Symbol Requirements: The operations being looked up must satisfy the Symbol trait (e.g., having a sym_name attribute) [1][2]. - Efficiency: lookupSymbolIn is designed to efficiently resolve symbols within the region of the specified symbol table operation [1][6]. - SymbolRefAttr: If you need to resolve complex or nested symbol references (e.g., path-like references), use the version of lookupSymbolIn that accepts a SymbolRefAttr [4][8].
Citations:
- 1: https://github.com/llvm/llvm-project/blob/main/mlir/docs/SymbolsAndSymbolTables.md
- 2: https://mlir.llvm.org/docs/SymbolsAndSymbolTables/
- 3: https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/IR/SymbolTable.h
- 4: https://mlir.llvm.org/doxygen/classmlir_1_1SymbolTable.html
- 5: https://mlir.llvm.org/doxygen/SymbolTable_8h_source.html
- 6: https://medium.com/@60b36t/a-primer-on-mlirs-symbol-table-996a0bc5728f
- 7: https://mlir.llvm.org/doxygen/classmlir_1_1SymbolTable-members.html
- 8: https://mlir.llvm.org/doxygen/SymbolTable_8cpp_source.html
Reject duplicate function names in startFunction.
func::FuncOp::create creates the function operation at the insertion point and does not enforce module symbol uniqueness. A second startFunction("f", ...) therefore leaves module-level symbol ambiguity for call("f", ...), since SymbolTable::lookupSymbolIn returns one matching symbol. Add an eager check using SymbolTable::lookupSymbolIn(module, name) before creating the funcOp.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mlir/lib/Dialect/QCO/Builder/QCOProgramBuilder.cpp` around lines 1414 - 1419,
In startFunction, check SymbolTable::lookupSymbolIn(module, name) before
func::FuncOp::create and reject the operation when a function with the same name
already exists. Preserve the existing insertion point, function creation, and
private visibility behavior for unique names.
| if (failed(applyPatternsGreedily(op, std::move(patterns)))) { | ||
| signalPassFailure(); | ||
| } | ||
|
|
||
| runQuantumArgumentPromotion(op); | ||
| runAuxiliaryQubitHoisting(op); | ||
| runQuantumFunctionBoundaryCommutation(op, symbolTable); | ||
| runQuantumFunctionBoundaryCommutation(op, symbolTable); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Return after signalPassFailure, and remove the duplicated commutation call.
Two defects are present in runOnOperation:
- Line 329 signals failure but execution continues into the three module-level transformations. These transformations then run on IR that the greedy driver could not bring to a fixed point.
runAuxiliaryQubitHoistingadditionally throwsstd::runtime_erroron failure, so a pass failure can escalate into an uncaught exception. - Lines 334 and 335 call
runQuantumFunctionBoundaryCommutationtwice with identical arguments. If a second iteration is intended, state that in a comment and drive it from a fixed-point loop. If it is a copy-paste error, remove one call.
🐛 Proposed fix
// Apply patterns in an iterative and greedy manner.
if (failed(applyPatternsGreedily(op, std::move(patterns)))) {
signalPassFailure();
+ return;
}
runQuantumArgumentPromotion(op);
runAuxiliaryQubitHoisting(op);
runQuantumFunctionBoundaryCommutation(op, symbolTable);
- runQuantumFunctionBoundaryCommutation(op, symbolTable);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (failed(applyPatternsGreedily(op, std::move(patterns)))) { | |
| signalPassFailure(); | |
| } | |
| runQuantumArgumentPromotion(op); | |
| runAuxiliaryQubitHoisting(op); | |
| runQuantumFunctionBoundaryCommutation(op, symbolTable); | |
| runQuantumFunctionBoundaryCommutation(op, symbolTable); | |
| } | |
| if (failed(applyPatternsGreedily(op, std::move(patterns)))) { | |
| signalPassFailure(); | |
| return; | |
| } | |
| runQuantumArgumentPromotion(op); | |
| runAuxiliaryQubitHoisting(op); | |
| runQuantumFunctionBoundaryCommutation(op, symbolTable); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mlir/lib/Dialect/QCO/Transforms/Optimizations/QuantumIPO.cpp` around lines
328 - 336, Update runOnOperation so it returns immediately after
signalPassFailure when applyPatternsGreedily fails, preventing subsequent
transformations from running on invalid IR. Remove the duplicated
runQuantumFunctionBoundaryCommutation call, keeping a single invocation with the
existing arguments.
| TEST_F(QCOQuantumIPOTest, specializeZeroArgumentDropsDiagonalGate) { | ||
| programBuilder.initialize(); | ||
| auto args = programBuilder.startFunction("f", {programBuilder.getQubitType()}, | ||
| {programBuilder.getQubitType()}); | ||
| programBuilder.endFunction({programBuilder.z(args[0])}); | ||
|
|
||
| auto q = programBuilder.allocQubit(); | ||
| auto results = programBuilder.call("f", {q}); | ||
| programBuilder.sink(results[0]); | ||
| module = programBuilder.finalize(); | ||
|
|
||
| referenceBuilder.initialize(); | ||
| // The original callee is retained, ... | ||
| auto refArgs = | ||
| referenceBuilder.startFunction("f", {referenceBuilder.getQubitType()}, | ||
| {referenceBuilder.getQubitType()}); | ||
| referenceBuilder.endFunction({referenceBuilder.z(refArgs[0])}); | ||
| // ... while the call is redirected to a specialization without the gate. | ||
| auto specArgs = referenceBuilder.startFunction( | ||
| "f_spec_zero_arg_0", {referenceBuilder.getQubitType()}, | ||
| {referenceBuilder.getQubitType()}); | ||
| referenceBuilder.endFunction({specArgs[0]}); | ||
|
|
||
| auto refQ = referenceBuilder.allocQubit(); | ||
| auto refResults = referenceBuilder.call("f_spec_zero_arg_0", {refQ}); | ||
| referenceBuilder.sink(refResults[0]); | ||
| reference = referenceBuilder.finalize(); | ||
|
|
||
| expectModuleMatchesReference(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add a zero-specialization test whose leading operation is a ResetOp.
operationIsNopOnZero in QuantumIPO.cpp accepts ResetOp at line 100, but no test covers that case. The existing tests use z, s, and a controlled gate only. The ResetOp path crashes, as described in my comment on QuantumIPO.cpp lines 192-205.
Add a case where the callee starts with a reset on the argument and the caller passes a freshly allocated qubit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mlir/unittests/Dialect/QCO/Transforms/Optimizations/test_qco_quantum_ipo.cpp`
around lines 102 - 131, Add a QCO quantum IPO zero-specialization test alongside
specializeZeroArgumentDropsDiagonalGate where the callee’s leading operation is
reset on its qubit argument and the caller passes a freshly allocated qubit.
Build the reference with the original reset-containing callee retained and a
zero-argument specialization that forwards the argument without the reset, then
invoke and sink the specialized result to validate the ResetOp path.
| TEST_F(QCOQuantumIPOTest, promoteTensorArgumentToQubitArgument) { | ||
| const auto tensorType = programBuilder.getQubitTensorType(2); | ||
|
|
||
| programBuilder.initialize(); | ||
| auto args = programBuilder.startFunction("f", {tensorType}, {tensorType}); | ||
| auto [tensorIn, inner] = programBuilder.qtensorExtract(args[0], 0); | ||
| inner = programBuilder.h(inner); | ||
| programBuilder.endFunction( | ||
| {programBuilder.qtensorInsert(inner, tensorIn, 0)}); | ||
|
|
||
| auto q0 = programBuilder.allocQubit(); | ||
| auto q1 = programBuilder.allocQubit(); | ||
| auto tensor = programBuilder.qtensorFromElements({q0, q1}); | ||
| auto results = programBuilder.call("f", {tensor}); | ||
| programBuilder.qtensorDealloc(results[0]); | ||
| module = programBuilder.finalize(); | ||
|
|
||
| referenceBuilder.initialize(); | ||
| auto refArgs = | ||
| referenceBuilder.startFunction("f", {referenceBuilder.getQubitType()}, | ||
| {referenceBuilder.getQubitType()}); | ||
| referenceBuilder.endFunction({referenceBuilder.h(refArgs[0])}); | ||
|
|
||
| auto refQ0 = referenceBuilder.allocQubit(); | ||
| auto refQ1 = referenceBuilder.allocQubit(); | ||
| auto refTensor = referenceBuilder.qtensorFromElements({refQ0, refQ1}); | ||
| // The caller extracts the promoted element, calls, and re-inserts it. | ||
| auto [refTensorIn, refExtracted] = | ||
| referenceBuilder.qtensorExtract(refTensor, 0); | ||
| auto refResults = referenceBuilder.call("f", {refExtracted}); | ||
| auto refInserted = | ||
| referenceBuilder.qtensorInsert(refResults[0], refTensorIn, 0); | ||
| referenceBuilder.qtensorDealloc(refInserted); | ||
| reference = referenceBuilder.finalize(); | ||
|
|
||
| expectModuleMatchesReference(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add a promotion test for a pass-through slot.
No test extracts a tensor element and re-inserts it without applying a gate. That shape reaches the use-after-erase path I flagged in QuantumArgumentPromotion.cpp at lines 229-246, because slot.insert.getScalar() and slot.extract.getResult() are then the same value.
Add a case that builds the callee as extract(arg, 0) followed directly by insert(qubit, tensor, 0), with the tensor as the only result.
As per coding guidelines: "Add or update automated tests for every behavioral code change."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mlir/unittests/Dialect/QCO/Transforms/Optimizations/test_qco_quantum_ipo.cpp`
around lines 546 - 582, Add a separate promotion test alongside
promoteTensorArgumentToQubitArgument where the callee extracts element 0 from
its tensor argument and immediately re-inserts that same qubit without applying
a gate, returning only the rebuilt tensor. Build the reference with the promoted
qubit passed through directly, then preserve the caller’s
extract/call/insert/deallocate structure and verify with
expectModuleMatchesReference.
Source: Coding guidelines
| uint64_t width = 0; | ||
| const auto type = it->second.type; | ||
| if (type->allowsDesignator()) { | ||
| width = type->getDesignator(); | ||
| } else { | ||
| const auto unsized = std::dynamic_pointer_cast<UnsizedType<uint64_t>>(type); | ||
| if (!unsized || unsized->type != qasm3::SingleQubit) { | ||
| return error("Cannot measure non-qubit type."); | ||
| } | ||
| width = 1; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 \
'struct InferredType|class InferredType|InferredType::error|isError' \
include src/qasm3Repository: munich-quantum-toolkit/core
Length of output: 13376
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Relevant TypeCheckPass.cpp sections:"
sed -n '136,178p' src/qasm3/passes/TypeCheckPass.cpp
echo
sed -n '360,392p' src/qasm3/passes/TypeCheckPass.cpp
echo
echo "Error calls around new non-qubit path:"
rg -n 'visitAssignmentStatement|visitIndexedIdentifierExpression|Non-qubit|bool b;|measure' src/qasm3 include/mqt-core/qasm3 -C 3Repository: munich-quantum-toolkit/core
Length of output: 16685
Guard the measure-to-assignment error before reading exprTy.type.
visitMeasureExpression() can return InferredType::error() for invalid measurement targets. Since TypeCheckPass::processStatement() only throws after statement.accept() completes, visitAssignmentStatement() then dereferences exprTy.type when that call produced a measure error. Add an if (exprTy.isError) return; guard after line 159.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/qasm3/passes/TypeCheckPass.cpp` around lines 378 - 388, In
visitAssignmentStatement(), immediately after evaluating the right-hand
expression and obtaining exprTy, check exprTy.isError and return before
dereferencing exprTy.type. Preserve the existing assignment type-checking flow
for non-error inferred types, including results from visitMeasureExpression().
| pow(0) @ x q0; | ||
| } | ||
| qubit q; | ||
| qubit[1] q; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Test the unsized qubit q; path.
🤖 AI text below 🤖
qubit[1] q; takes the sized DesignatedType path and returns before src/qasm3/Parser.cpp Lines 910-914. It does not exercise UnsizedTy::SingleQubit or the new width-one handling in src/qasm3/Importer.cpp and src/qasm3/passes/TypeCheckPass.cpp.
Change this case to the unsized declaration and add a separate measurement case.
Proposed test change
-qubit[1] q;
+qubit q;As per coding guidelines, test/**/* requires automated tests for every behavioral code change in the corresponding subsystem test tree.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/ir/test_qasm3_parser.cpp` at line 209, Change the test declaration from
sized `qubit[1] q;` to unsized `qubit q;` so it exercises the
`UnsizedTy::SingleQubit` path through the parser, importer, and type checker,
and add a separate measurement test case covering that unsized qubit behavior.
Source: Coding guidelines
Description
This PR introduces the IPO passes from the corresponding paper to
mqt-cc.It includes:
AI Notice: Gemini 3.1 Pro and Claude Opus 5.0 were used for the generation of some code.
Checklist
If PR contains AI-assisted content:
🤖 *AI text below* 🤖(titles are exempt).